ci/prow/e2e-incluster-build (2)

45-45: Add error handling for timeout

The script should handle cases where the timeout is reached. Currently, if the resources can't be applied within 1 minute, the script will fail without a clear error message.

-timeout 1m bash -c 'until oc apply -k ci/e2e; do sleep 3; done'
+if ! timeout 1m bash -c 'until oc apply -k ci/e2e; do sleep 3; done'; then
+  echo "ERROR: Failed to apply resources after 1 minute of retries"
+  exit 1
+fi

---

Line range hint 48-51: Simplify and improve pod name matching

The current jq queries for matching pod names could be simplified and made more robust by:

1. Using explicit label selectors with oc get pods
2. Simplifying the jq query

-timeout 1m bash -c 'until oc get pods -o json | jq -er ".items[].metadata.name | select(.? | match(\"build\"))"; do sleep 1; done'
-POD_NAME=$(oc get pods -o json | jq -r '.items[].metadata.name | select(.? | match("build"))')
+timeout 1m bash -c 'until oc get pods -l job-name=kmm-ci-build -o name; do sleep 1; done'
+POD_NAME=$(oc get pods -l job-name=kmm-ci-build -o name | sed 's|pod/||')

-timeout 1m bash -c 'until oc get pods -o json | jq -er ".items[].metadata.name | select(.? | match(\"sign\"))"; do sleep 1; done'
-POD_NAME=$(oc get pods -o json | jq -r '.items[].metadata.name | select(.? | match("sign"))')
+timeout 1m bash -c 'until oc get pods -l job-name=kmm-ci-sign -o name; do sleep 1; done'
+POD_NAME=$(oc get pods -l job-name=kmm-ci-sign -o name | sed 's|pod/||')

Also applies to: 54-57

ci/prow/e2e-hub-spoke-incluster-build (3)

50-50: Document timeout values

The script uses various timeout values (1m, 3m, 15m) for different operations. Consider adding comments explaining the rationale behind these timeout values to help maintainers understand and adjust them if needed.

+# Retry applying resources for up to 1 minute to handle webhook service availability
 timeout 1m bash -c 'until oc apply -k ci/e2e-hub; do sleep 3; done'

---

Line range hint 52-54: Standardize build wait patterns

The build wait patterns could be standardized with the pod wait patterns from the other script for consistency.

-timeout 1m bash -c 'until oc -n ${HUB_OPERATOR_NAMESPACE} get builds -o json | jq -er ".items[].metadata.name | select(.? | match(\"build\"))"; do sleep 1; done'
-export build_build=$(oc -n ${HUB_OPERATOR_NAMESPACE} get builds -o json | jq -r '.items[].metadata.name | select(.? | match("build"))')
+timeout 1m bash -c 'until oc -n ${HUB_OPERATOR_NAMESPACE} get builds -l job=kmm-ci-build -o name; do sleep 1; done'
+export build_build=$(oc -n ${HUB_OPERATOR_NAMESPACE} get builds -l job=kmm-ci-build -o name | sed 's|build/||')

-timeout 1m bash -c 'until oc -n ${HUB_OPERATOR_NAMESPACE} get builds -o json | jq -er ".items[].metadata.name | select(.? | match(\"sign\"))"; do sleep 1; done'
-export sign_build=$(oc -n ${HUB_OPERATOR_NAMESPACE} get builds -o json | jq -r '.items[].metadata.name | select(.? | match("sign"))')
+timeout 1m bash -c 'until oc -n ${HUB_OPERATOR_NAMESPACE} get builds -l job=kmm-ci-sign -o name; do sleep 1; done'
+export sign_build=$(oc -n ${HUB_OPERATOR_NAMESPACE} get builds -l job=kmm-ci-sign -o name | sed 's|build/||')

Also applies to: 57-59

---

50-50: Consider extracting common retry patterns

Both scripts implement similar retry patterns for oc apply and resource waiting. Consider extracting these patterns into helper functions in a shared script to reduce duplication and ensure consistency.

Example shared functions:

# helpers.sh
wait_for_resource() {
    local namespace=$1
    local resource_type=$2
    local selector=$3
    local timeout=${4:-1m}
    
    if ! timeout "${timeout}" bash -c "until oc -n ${namespace} get ${resource_type} ${selector} &>/dev/null; do sleep 1; done"; then
        echo "ERROR: Timeout waiting for ${resource_type} in namespace ${namespace}"
        return 1
    fi
}

retry_apply() {
    local path=$1
    local timeout=${2:-1m}
    
    if ! timeout "${timeout}" bash -c "until oc apply -k ${path}; do sleep 3; done"; then
        echo "ERROR: Failed to apply resources at ${path} after ${timeout} of retries"
        return 1
    fi
}

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

ci/prow/e2e-hub-spoke-incluster-build (1)

Line range hint 1-54: Consider architectural improvements for better reliability.

The script has several areas where reliability could be improved:

1. Inconsistent timeout durations (1m vs 3m vs 15m)
2. Limited error handling and reporting
3. No cleanup on failure

Consider these improvements:

1. Define timeout constants at the top of the script:

readonly RESOURCE_TIMEOUT="5m"
readonly BUILD_TIMEOUT="10m"
readonly ACM_TIMEOUT="15m"

2. Add a trap for cleanup:

cleanup() {
  # Add cleanup logic here
  echo "Cleaning up resources..."
}
trap cleanup EXIT

3. Add error reporting function:

report_error() {
  local component=$1
  local message=$2
  echo "Error in ${component}: ${message}"
  # Add logging or metric collection
}

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro